refactor(scene): 将场景路由异步方法改为await调用#43
Conversation
- 将SceneRouterBase中的AfterChange方法改为异步方法AfterChangeAsync - 修改SceneRouterBase中的场景切换流程使用await调用异步方法 - 移除后台线程执行,直接使用同步await调用 - 更新日志记录逻辑,优化调试信息输出 - 简化异常处理流程,统一异步执行模式
Reviewer's GuideRefactors the scene and UI routing systems to use fully async/await-based transition flows, removes background-thread fire-and-forget execution, and improves logging and documentation around transitions and UI navigation APIs. Sequence diagram for async UI PushAsync flowsequenceDiagram
actor Caller
participant UiRouterBase
participant UiTransitionPipeline
participant Guard1 as IUiRouteGuard
participant Guard2 as IUiRouteGuard
participant UiFactory as IUiFactory
participant Page as IUiPageBehavior
Caller->>UiRouterBase: PushAsync(uiKey, param, policy)
activate UiRouterBase
UiRouterBase->>UiRouterBase: CreateEvent(uiKey, Push, policy, param)
UiRouterBase->>UiTransitionPipeline: ExecuteAroundAsync(event, inner)
activate UiTransitionPipeline
UiTransitionPipeline->>UiTransitionPipeline: FilterAndSortHandlers(event, BeforeChange)
UiTransitionPipeline-->>UiRouterBase: await inner()
deactivate UiTransitionPipeline
UiRouterBase->>UiRouterBase: BeforeChangeAsync(event)
activate UiRouterBase
UiRouterBase->>UiTransitionPipeline: ExecuteAsync(event, BeforeChange)
activate UiTransitionPipeline
UiTransitionPipeline-->>UiRouterBase: completed
deactivate UiTransitionPipeline
deactivate UiRouterBase
UiRouterBase->>UiRouterBase: DoPushPageInternalAsync(uiKey, param, policy)
activate UiRouterBase
UiRouterBase->>Guard1: ExecuteEnterGuardsAsync(uiKey, param)
activate Guard1
Guard1-->>UiRouterBase: bool
deactivate Guard1
UiRouterBase->>Guard2: ExecuteEnterGuardsAsync(uiKey, param)
activate Guard2
Guard2-->>UiRouterBase: bool
deactivate Guard2
UiRouterBase->>UiFactory: Create(uiKey)
activate UiFactory
UiFactory-->>UiRouterBase: Page
deactivate UiFactory
UiRouterBase->>Page: OnEnter(param)
UiRouterBase->>Page: OnShow()
deactivate UiRouterBase
UiRouterBase->>UiRouterBase: AfterChangeAsync(event)
activate UiRouterBase
UiRouterBase->>UiTransitionPipeline: ExecuteAsync(event, AfterChange)
activate UiTransitionPipeline
UiTransitionPipeline-->>UiRouterBase: completed
deactivate UiTransitionPipeline
deactivate UiRouterBase
UiTransitionPipeline-->>Caller: ExecuteAroundAsync completed
UiRouterBase-->>Caller: ValueTask completed
Sequence diagram for async SceneRouterBase AfterChangeAsyncsequenceDiagram
actor GameCode
participant SceneRouterBase
participant SceneTransitionPipeline
GameCode->>SceneRouterBase: ReplaceAsync(sceneKey, param)
activate SceneRouterBase
SceneRouterBase->>SceneRouterBase: CreateEvent(sceneKey, Replace)
SceneRouterBase->>SceneTransitionPipeline: ExecuteAroundAsync(event, inner)
activate SceneTransitionPipeline
SceneTransitionPipeline-->>SceneRouterBase: await inner()
deactivate SceneTransitionPipeline
SceneRouterBase->>SceneRouterBase: BeforeChangeAsync(event)
activate SceneRouterBase
SceneRouterBase->>SceneTransitionPipeline: ExecuteAsync(event, BeforeChange)
activate SceneTransitionPipeline
SceneTransitionPipeline-->>SceneRouterBase: completed
deactivate SceneTransitionPipeline
deactivate SceneRouterBase
SceneRouterBase->>SceneRouterBase: ClearInternalAsync()
activate SceneRouterBase
SceneRouterBase-->>SceneRouterBase: scenes cleared
deactivate SceneRouterBase
SceneRouterBase->>SceneRouterBase: PushInternalAsync(sceneKey, param)
activate SceneRouterBase
SceneRouterBase-->>SceneRouterBase: new scene active
deactivate SceneRouterBase
SceneRouterBase->>SceneRouterBase: AfterChangeAsync(event)
activate SceneRouterBase
SceneRouterBase->>SceneTransitionPipeline: ExecuteAsync(event, AfterChange)
activate SceneTransitionPipeline
SceneTransitionPipeline-->>SceneRouterBase: completed
deactivate SceneTransitionPipeline
deactivate SceneRouterBase
SceneTransitionPipeline-->>GameCode: ExecuteAroundAsync completed
SceneRouterBase-->>GameCode: ReplaceAsync ValueTask completed
Class diagram for async UI router API refactorclassDiagram
class IUiRouter {
<<interface>>
+ValueTask PushAsync(string uiKey, IUiPageEnterParam param, UiTransitionPolicy policy)
+ValueTask PushAsync(IUiPageBehavior page, IUiPageEnterParam param, UiTransitionPolicy policy)
+ValueTask PopAsync(UiPopPolicy policy)
+ValueTask ReplaceAsync(string uiKey, IUiPageEnterParam param, UiPopPolicy popPolicy, UiTransitionPolicy pushPolicy)
+ValueTask ReplaceAsync(IUiPageBehavior page, IUiPageEnterParam param, UiPopPolicy popPolicy, UiTransitionPolicy pushPolicy)
+ValueTask ClearAsync()
}
class AbstractSystem {
}
class UiRouterBase {
-ILogger Log
-List~IUiRouteGuard~ _guards
-Dictionary~UiLayer, Dictionary~string, IUiPageBehavior~~ _layers
-UiTransitionPipeline _pipeline
-Stack~IUiPageBehavior~ _stack
-IUiFactory _factory
-int _instanceCounter
-IUiRoot _uiRoot
+void RegisterHandler(IUiTransitionHandler handler, UiTransitionHandlerOptions options)
+void UnregisterHandler(IUiTransitionHandler handler)
+void BindRoot(IUiRoot root)
+ValueTask PushAsync(string uiKey, IUiPageEnterParam param, UiTransitionPolicy policy)
+ValueTask PushAsync(IUiPageBehavior page, IUiPageEnterParam param, UiTransitionPolicy policy)
+ValueTask PopAsync(UiPopPolicy policy)
+ValueTask ReplaceAsync(string uiKey, IUiPageEnterParam param, UiPopPolicy popPolicy, UiTransitionPolicy pushPolicy)
+ValueTask ReplaceAsync(IUiPageBehavior page, IUiPageEnterParam param, UiPopPolicy popPolicy, UiTransitionPolicy pushPolicy)
+ValueTask ClearAsync()
+string PeekKey()
+IUiPageBehavior Peek()
+bool IsTop(string uiKey)
+bool Contains(string uiKey)
+int Count
+UiHandle Show(string uiKey, UiLayer layer, IUiPageEnterParam param)
+UiHandle Show(IUiPageBehavior page, UiLayer layer)
+void Hide(UiHandle handle, UiLayer layer, bool destroy)
+void Resume(UiHandle handle, UiLayer layer)
+void ClearLayer(UiLayer layer, bool destroy)
+UiHandle GetFromLayer(UiHandle handle, UiLayer layer)
+IReadOnlyList~UiHandle~ GetAllFromLayer(string uiKey, UiLayer layer)
+bool HasVisibleInLayer(UiHandle handle, UiLayer layer)
+void HideByKey(string uiKey, UiLayer layer, bool destroy, bool hideAll)
+void AddGuard(IUiRouteGuard guard)
+void AddGuard~T~()
+void RemoveGuard(IUiRouteGuard guard)
-string GenerateInstanceId()
-UiHandle ShowInternal(IUiPageBehavior page, UiLayer layer, IUiPageEnterParam param)
-UiTransitionEvent CreateEvent(string toUiKey, UiTransitionType type, UiTransitionPolicy policy, IUiPageEnterParam param)
-Task BeforeChangeAsync(UiTransitionEvent event)
-Task AfterChangeAsync(UiTransitionEvent event)
-Task DoPushPageInternalAsync(string uiKey, IUiPageEnterParam param, UiTransitionPolicy policy)
-void DoPushPageInternal(IUiPageBehavior page, IUiPageEnterParam param, UiTransitionPolicy policy)
-void DoPopInternal(UiPopPolicy policy)
-void DoClearInternal(UiPopPolicy policy)
-Task~bool~ ExecuteEnterGuardsAsync(string uiKey, IUiPageEnterParam param)
-Task~bool~ ExecuteLeaveGuardsAsync(string uiKey)
}
class UiTransitionPipeline {
-List~IUiTransitionHandler~ _handlers
+void RegisterHandler(IUiTransitionHandler handler, UiTransitionHandlerOptions options)
+void UnregisterHandler(IUiTransitionHandler handler)
+Task ExecuteAroundAsync(UiTransitionEvent event, Func~Task~ inner)
+Task ExecuteAsync(UiTransitionEvent event, UiTransitionPhases phases)
}
class IUiRouteGuard {
<<interface>>
+Task~bool~ CanEnterAsync(string uiKey, IUiPageEnterParam param)
+Task~bool~ CanLeaveAsync(string uiKey)
}
class IUiPageBehavior {
<<interface>>
+string Key
+bool AllowReenter
+void OnEnter(IUiPageEnterParam param)
+void OnPause()
+void OnResume()
+void OnLeave()
+void OnShow()
+void OnHide()
}
class IUiFactory {
<<interface>>
+IUiPageBehavior Create(string uiKey)
}
class IUiRoot {
<<interface>>
}
class UiTransitionEvent {
}
class UiHandle {
+string InstanceId
+string UiKey
}
IUiRouter <|.. UiRouterBase
AbstractSystem <|-- UiRouterBase
UiRouterBase --> UiTransitionPipeline : uses
UiRouterBase --> IUiRouteGuard : manages
UiRouterBase --> IUiPageBehavior : manages stack
UiRouterBase --> IUiFactory : creates pages
UiRouterBase --> IUiRoot : binds
UiRouterBase --> UiTransitionEvent : creates
UiRouterBase --> UiHandle : returns
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- Converting
AfterChange/AfterChangeAsyncfrom fire-and-forget background execution to awaited calls changes both threading and error propagation semantics; double-check that callers and pipeline handlers are safe with the extra latency and that any previously-ignored exceptions are now either handled or intentionally allowed to bubble. - The
IUiRouterAPI now exposes only async versions (PushAsync,PopAsync,ReplaceAsync,ClearAsync) while several layer-related operations likeShow/Hideremain synchronous; consider whether these should also be async for consistency if they may perform asynchronous work (e.g., animations or async loading). - The scene transition logging in
SceneTransitionPipeline.ExecuteAsyncnow logsFromSceneKeyeven when null (as "None"), which may be a behavior change from the previous conditional logging; confirm that downstream log analysis or tooling does not rely on the previous omission of entries without aFromSceneKey.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Converting `AfterChange`/`AfterChangeAsync` from fire-and-forget background execution to awaited calls changes both threading and error propagation semantics; double-check that callers and pipeline handlers are safe with the extra latency and that any previously-ignored exceptions are now either handled or intentionally allowed to bubble.
- The `IUiRouter` API now exposes only async versions (`PushAsync`, `PopAsync`, `ReplaceAsync`, `ClearAsync`) while several layer-related operations like `Show/Hide` remain synchronous; consider whether these should also be async for consistency if they may perform asynchronous work (e.g., animations or async loading).
- The scene transition logging in `SceneTransitionPipeline.ExecuteAsync` now logs `FromSceneKey` even when null (as "None"), which may be a behavior change from the previous conditional logging; confirm that downstream log analysis or tooling does not rely on the previous omission of entries without a `FromSceneKey`.
## Individual Comments
### Comment 1
<location> `GFramework.Game/scene/SceneRouterBase.cs:456-463` </location>
<code_context>
/// </summary>
/// <param name="event">场景转换事件。</param>
- private void AfterChange(SceneTransitionEvent @event)
+ private async Task AfterChangeAsync(SceneTransitionEvent @event)
{
Log.Debug("AfterChange phases started: {0}", @event.TransitionType);
- _ = Task.Run(async () =>
- {
- try
- {
- await _pipeline.ExecuteAsync(@event, SceneTransitionPhases.AfterChange);
- Log.Debug("AfterChange phases completed: {0}", @event.TransitionType);
- }
- catch (Exception ex)
</code_context>
<issue_to_address>
**question (bug_risk):** Scene AfterChangeAsync now runs synchronously in the transition flow rather than fire-and-forget, which may affect latency and error handling.
Previously, `AfterChange` ran via `Task.Run` and isolated failures (log + swallow). Now `AfterChangeAsync` is awaited, so scene transitions won’t complete until all handlers finish, and any exception will surface to the caller and can fail/cancel the transition. Please confirm if this change in behavior is intentional. If you don’t want `AfterChange` failures to abort navigation, consider adding isolation around the pipeline execution (while avoiding `Task.Run` if possible), so errors remain logged but non-fatal.
</issue_to_address>
### Comment 2
<location> `GFramework.Game/ui/UiRouterBase.cs:612-613` </location>
<code_context>
+ /// <summary>
+ /// 执行过渡前阶段
+ /// </summary>
+ /// <param name="event">UI过渡事件</param>
+ private async Task BeforeChangeAsync(UiTransitionEvent @event)
{
Log.Debug("BeforeChange phases started: {0}", @event.TransitionType);
</code_context>
<issue_to_address>
**nitpick (typo):** XML doc param name doesn’t match the actual parameter name (`event` vs `@event`).
This mismatch will generate XML documentation warnings and may confuse tooling. Update the XML to `<param name="@event">` to match the parameter name (or rename the parameter if you want to avoid the escaped identifier).
Suggested implementation:
```csharp
/// <summary>
/// 执行过渡前阶段
/// </summary>
/// <param name="@event">UI过渡事件</param>
private async Task BeforeChangeAsync(UiTransitionEvent @event)
```
```csharp
/// <summary>
/// 执行过渡后阶段
/// </summary>
/// <param name="@event">UI过渡事件</param>
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| private async Task AfterChangeAsync(SceneTransitionEvent @event) | ||
| { | ||
| Log.Debug("AfterChange phases started: {0}", @event.TransitionType); | ||
| _ = Task.Run(async () => | ||
| { | ||
| try | ||
| { | ||
| await _pipeline.ExecuteAsync(@event, SceneTransitionPhases.AfterChange); | ||
| Log.Debug("AfterChange phases completed: {0}", @event.TransitionType); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| Log.Error("AfterChange phases failed: {0}, Error: {1}", | ||
| @event.TransitionType, ex.Message); | ||
| } | ||
| }); | ||
| await _pipeline.ExecuteAsync(@event, SceneTransitionPhases.AfterChange); | ||
| Log.Debug("AfterChange phases completed: {0}", @event.TransitionType); | ||
| } | ||
|
|
||
| /// <summary> |
There was a problem hiding this comment.
question (bug_risk): Scene AfterChangeAsync now runs synchronously in the transition flow rather than fire-and-forget, which may affect latency and error handling.
Previously, AfterChange ran via Task.Run and isolated failures (log + swallow). Now AfterChangeAsync is awaited, so scene transitions won’t complete until all handlers finish, and any exception will surface to the caller and can fail/cancel the transition. Please confirm if this change in behavior is intentional. If you don’t want AfterChange failures to abort navigation, consider adding isolation around the pipeline execution (while avoiding Task.Run if possible), so errors remain logged but non-fatal.
| /// <param name="event">UI过渡事件</param> | ||
| private async Task BeforeChangeAsync(UiTransitionEvent @event) |
There was a problem hiding this comment.
nitpick (typo): XML doc param name doesn’t match the actual parameter name (event vs @event).
This mismatch will generate XML documentation warnings and may confuse tooling. Update the XML to <param name="@event"> to match the parameter name (or rename the parameter if you want to avoid the escaped identifier).
Suggested implementation:
/// <summary>
/// 执行过渡前阶段
/// </summary>
/// <param name="@event">UI过渡事件</param>
private async Task BeforeChangeAsync(UiTransitionEvent @event) /// <summary>
/// 执行过渡后阶段
/// </summary>
/// <param name="@event">UI过渡事件</param>- 添加空值检查避免FromUiKey为null时的日志记录错误 - 确保流水线执行时参数安全处理
Summary by Sourcery
Refactor UI and scene routing to use fully asynchronous transition pipelines and improve documentation and logging around UI and scene transitions.
Enhancements: